Address SPDD safe-output spec gaps - #55441

Fixed in a4778c5 and follow-up commits: replace_label now fetches pre-write state, revalidates both labels against live policy, recomputes the label payload, and then calls setLabels inside each retry attempt. Policy rejections are explicitly non-retryable.

Fixed in 6b88f1c: specs/replace-label-spec.md now defines explicit 404 and 5xx/timeout/transport handling in Section 7.

Fixed in 6b88f1c: the cross-reference is now backed by explicit 404 and 5xx/timeout/transport rules in replace-label-spec.md Section 7.

[/tdd] The "mid-flight blocklist change" test simulates mutation via a shared JS array reference, not an actual config-reload path. This means the test passes for the wrong reason: it's testing JS reference semantics rather than proving a real re-validation occurs before the write.

it("should reject label_to_add before setLabels when it is blocked", async () => {
  let setLabelsCalls = 0;
  mockGithub.rest.issues.setLabels = async () => { setLabelsCalls++; return { data: [] }; };

  const handler = await main({ allowed_add: ["done"], blocked: ["done"] });
  const result = await handler({ label_to_remove: "in-progress", label_to_add: "done" }, {});

  expect(result.success).toBe(false);
  expect(result.error).toContain("blocked pattern");
  expect(setLabelsCalls).toBe(0);
});

@copilot please address this.

Fixed in a47dd6e: the JS regression now models a config reload by replacing config.blocked during the second GET, and the handler re-reads current policy at the pre-write gate.

[/codebase-design] formalRunReplaceLabel now has 6 parameters including a beforeWrite func() []string callback — a significant complexity increase for a helper that previously had 2. This makes the formal-model tests harder to read at a glance.

type formalRunOpts struct {
    LabelToAdd  string
    AllowedAdd  []string
    Blocked     []string
    BeforeWrite func() []string
    OnWrite     func()
}

func formalRunReplaceLabel(staged bool, opts formalRunOpts) formalReplaceLabelOutcome { ... }

@copilot please address this.

Fixed in 6b88f1c: formalRunReplaceLabel now takes a formalRunReplaceLabelOpts struct so call sites are self-documenting.

[/tdd] TestFormalBlockedLabelAddedViaReplaceLabelMidAllowlistChange_NoWriteAPI initialises blocked := []string{} and then passes append(blocked, "done") from beforeWrite — but append on an empty slice returns a new slice, so the outer blocked variable is still empty after beforeWrite returns. The formal model correctly rejects the label because formalRunReplaceLabel reassigns its local blocked parameter, but the test name implies mid-flight mutation of the original slice, which isn't what happens.

// beforeWrite returns a new blocked list — simulates a policy reload
func() []string { return []string{"done"} },

@copilot please address this.

Fixed in 6b88f1c: the formal test is renamed for the mid-blocklist scenario and BeforeWrite now returns a new blocked list to model policy reload explicitly.

[/codebase-design] RL-049a says re-check MUST happen "immediately before invoking PUT .../labels" but the implementation inserts the check after newLabelNames is computed (line ~204) and before core.info + fetchIssueState — which itself makes additional read API calls. Those reads mean the check is not the last operation before the write.

@copilot please address this.

Fixed across a4778c5 and follow-up commits: the final policy gate now runs inside each retry callback after the fresh state read and immediately before setLabels.